Portable MFIter Loops w/ Threading - #610
Open
ax3l wants to merge 2 commits into
Open
Conversation
Adds what is needed to write one field kernel that runs CPU-serial, CPU-threaded and on GPU, mirroring the C++ MFIter + ParallelFor idiom, and fixes two iteration bugs found along the way. Iteration - MFIter gains __iter__, so `for mfi in amr.MFIter(mf, info):` works and a tiled MFIter is usable at all. - Iteration now yields a generator whose `finally` finalizes the iterator. Previously __next__ returned self, so the loop variable pinned the iterator and `break` left it live: the next MFIter construction tripped AMREX_ALWAYS_ASSERT(depth == 1) and aborted the process, and on GPU the Gpu::streamSynchronize() in Finalize() was skipped. ParIter had the same bug (it derives from MFIter) and is fixed the same way. Kernels - Array4.__call__(bx, di, dj, dk, comp) gives a Box-shaped view in AMReX global index space, so ghost-cell stencils are writable and correct under tiling, where a whole-array expression would otherwise be applied once per tile to the entire fab. - TilingIfNotGPU()/MultiFab.tiles() tile on CPU and never on GPU. Unlike C++, the tile size has no default: AMReX's (1024000,8,8) yields thousands of tiny work units in Python and measures slower than serial. - for_each_tile() is the MFIter loop as a decorator, so the call site reads in the same order as the C++ block, with an optional thread pool. - MultiFab.ix_type mirrors C++ mf.ixType().toIntVect(). Note that iter(mfab) is now a generator rather than an MFIter; the two tests asserting iter(mfab).length ask amr.MFIter(mfab) instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Similar to here / alternate to explore: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lets you write one field kernel that runs CPU-serial, CPU-threaded and on GPU,
mirroring the C++
MFIter+ParallelForidiom. Motivated by#607, which asked how OpenMP
parallelizes a pyAMReX
MFIterloop; the honest answer is that it does not andcannot w/o making at least a subset of the AMReX public APIs threadsafe #614, so this provides what people actually need instead.
The kernel block
C++ today, e.g. WarpX
ComputeDivE.cpp:The same thing in Python:
The decorator line stands in for the pragma and the
MFIterline, its parameterlist for the
tileboxandarray(mfi)extractions, and its body for theParallelForlambda. Same reading order, no nesting.User-facing API
Array4.__call__(bx, di=0, dj=0, dk=0, comp=0)a(bx, di=-1)is the analogue of C++a(i-1,j,k)overbx. Returns anAMREX_SPACEDIM-dimensional NumPy/CuPy/dpnp view.MultiFab.tiles(tile=None)iMultiFabtoo.MultiFab.ix_typeIntVectindex type; mirrors C++mf.ixType().toIntVect().amr.TilingIfNotGPU(tile=None)MFItInfothat tiles on CPU and never on GPU.amr.for_each_tile(mfab, *others, tile=None, threads=1)MFIterloop as a decorator, with an optional thread pool.for mfi in amr.MFIter(mf, info):MFItergained__iter__, so an explicitly constructed (e.g. tiled) iterator is finally usable.Array4.__call__is the load-bearing one.to_xp()is a locally 0-based view ofthe whole fab; indexing by
Boxin global index space is what makes ghost-cellstencils writable, and what makes them correct under tiling, where a
whole-array expression would otherwise be applied once per tile to the entire
fab.
Two bug fixes
Leaving an
MFIterloop early aborted the process.__next__returnedself, so the loop variable pinned the iterator andfinalize()was onlyreached on the
StopIterationpath. Afterbreak, the nextMFIterconstruction tripped
AMREX_ALWAYS_ASSERT(depth == 1)and killed the process;on GPU the
Gpu::streamSynchronize()inFinalize()was skipped too.Iteration now yields a generator whose
finallyfinalizes on break, return andexceptions alike -- the coverage C++ gets from
~MFIter()at scope exit.ParIterhad the identical bug (it derives fromMFIterand inherits itsFinalize()) and is fixed the same way.Behavior change
iter(mfab)is now a generator rather than theMFIteritself. Useamr.MFIter(mfab)if you want the object; the two tests that assertediter(mfab).lengthwere updated.Notes on the design
TilingIfNotGPU()deliberately diverges from C++: the tile size has nodefault, tiling is opt-in. AMReX's
(1024000,8,8)is sized for OpenMP, wherea tile is nearly free; in Python each tile costs a loop iteration plus an array
view per field, and with that tile size a representative kernel measured 0.4x,
i.e. slower than serial. A large default would instead be a silent no-op
against typical 16-64^3 boxes. Tiling pays off in one case: fewer boxes than
threads, feeding the thread pool.
threads=uses a thread pool, which parallelizes because NumPy/CuPy release theGIL for the array operations a kernel body is made of. A single serial
MFIterpass snapshots the per-tile arguments first; that is required, not an
optimization, because the
MFItermutates in place and yields itself, and AMReXpermits only one live
MFIterat a time. It is a separate pool from anAMREX_OMP=ONbuild's -- using both oversubscribes the node.Measured on 256^3 / 64 tiles with a compute-bound kernel:
1.0 / 1.7 / 2.9 / 4.6 / 5.4x at 1/2/4/8/20 threads.
Explicit non-goals, now stated in the docs: there is no
ParallelFor(devicelambdas cannot be written in Python) and no OpenMP region. Portability here means
array-expression portability across NumPy/CuPy/dpnp, not scalar-kernel
portability.
Docs
New "Portable Kernels, OpenMP and Threading" section in
docs/source/usage/compute.rst-- the page previously never mentioned OpenMP,which is what prompted #607. Covers the recipe, on-node parallelism options,
tiling guidance, the GPU stream pool and synchronization, and the non-goals.
Testing
AMReX_SPACEDIM="1;2;3",AMReX_OMP=ON:ctestgreen; the newArray4.__call__slicing verified per dimensionality (anArray4is always 4Dwith extent-1 padding, so 1D/2D would otherwise index
kwithcomp-- a bugthe 3D suite cannot catch).
sm_86): 311 passed, 32 skipped.fp64): all new tests pass,
amr.xpresolves to dpnp, views are realdpnp_arrays.branch of ImpactX
ForceFromSelfFields.cpp; with linear input fields thecentral differences are exact. Untiled, tiled and threaded runs agree.
Known pre-existing SYCL flake, unrelated to this PR:
test_podvector.pyerrorsintermittently via the DLPack finalize guard. Measured with an equal-N A/B --
this branch touches no C++, so the
.sois identical and only the Python filesare swapped: 12/40 failures on this branch vs 11/40 on
development, i.e.indistinguishable.
test_imfab_numpyon SYCL is pre-existing by the same method.Both are worth a separate issue.
🤖 Generated with Claude Code